Add WDT Library #242

adamgarbo · 2020-08-01T15:17:12Z

After our recent discussions regarding #237 by @stephenf7072 , I was motivated to get the ball rolling with writing a WDT library the Apollo3 Core. I started working with the WDT HAL earlier this year, and have been meaning put a library together for some time.

Library summary:
For the most part, this library is simply a wrapper of the Ambiq HAL. As we know, the HAL can be scary, so the watchdog functions can now be called using basic commands such as start, stop, restart, etc.

Where I needed to bypass the HAL and switch to writing registers directly was with the watchdog configuration functionality. Normally, when configuring the WDT with the HAL, you need to create a specific am_hal_wdt_config_t structure, specify your clock divider and tick values and then pass this structure to the HAL function am_hal_wdt_init, which actually does the configuration. Rather than dealing with structures, addresses and pointers, I opted to write a few of simple functions (e.g. configure(), setClock(), setInterrupt(), setReset(), to have greater overall control of these registers.

Default configuration: There is a globally declared am_hal_wdt_config_t watchdogConfig structure in the header file, which is used for the initial WDT configuration. This defaults the LFRC clock divider to 16 Hz, the interrupt period to 4 seconds and reset period to 8 seconds. For users of either Atmega328p or SAMD21 watchdogs, these periods will be familiar, though are ultimately arbitrarily and able to be changed.

All examples tested with a SparkFun Redboard Artemis. A test program (not included) was also run to iterate the WDT through every clock divider, interrupt and reset tick combination to ensure correct timings.

My hope is that other users will identify additional required functionality or make contributions themselves. I welcome your feedback!

Cheers,
Adam

Examples:

1. Example1_WDT_Basic

An example demonstrating the basic use of the watchdog timer. It will demonstrate to the user watchdog interrupt and reset triggers. Based on the Ambiq HAL watchdog.c example.

2. Example2_WDT_Config

A more advanced version of Example 1, which demonstrates how to manually configure the watchdog timer clock divider, interrupt ticks and reset ticks. Comments provide examples on how to calculate the desired interrupt and/or reset period.

3. Example3_WDT_LowPower

A more advanced example, which makes use of both the Artemis RTC and WDT libraries. This example demonstrates how to use the WDT in conjunction with RTC deep sleep. Draws from RTC example Example6_LowPower_Alarm.

…in().

Create keywords.txt

Bringing up to date with release1.1.2

I hate to undo even a little bit of Adam's good work, but in the interests of a robust core, I think it's best the references to time.h are removed, and the Epoch set/get along with it. RTC.cpp included <time.h>, which caused a compilation error when these are also included in code: #include <Time.h> #include <TimeLib.h> Some possible issues with Window lack of case sensitivity, refer here: https://forum.arduino.cc/index.php?topic=451360.0 Also with time.h (or Time.h?) being an optional extra install library to the Arduino IDE, best to leave it out of the core, especially if it can cause compilation errors.

This reverts commit ece522d.

this keeps room around for the legacy BLE led example

adamgarbo · 2020-08-01T15:30:38Z

A quick note that the failed checks are due to: fatal error: WDT.h: No such file or directory

Is this perhaps due to using #include <WDT.h> instead of #include "WDT.h" in the examples?

libraries/WDT/src/WDT.cpp

libraries/WDT/examples/Example3_WDT_LowPower/Example3_WDT_LowPower.ino

libraries/WDT/examples/Example2_WDT_Config/Example2_WDT_Config.ino

stephenf7072 · 2020-08-04T04:49:29Z

So yes, I've been through and commented, I thought that was just on the files, but it's made a whole lot of comments on the main conversations - any Github tips?

Basically - excellent work, I only had suggestions for:

Add a call to am_hal_interrupt_master_enable()? (although perhaps it isn't necessary)
Add error checking for wrong values passed into setting the clock frequency
Add excessive comments about what can be passed into setting the clock frequency

Added simplied defintions and error checking to setClock() and configure(). More detailed comments.

adamgarbo · 2020-08-04T13:55:49Z

Thanks @stephenf7072!

I believe I've addressed all of your comments in 38a241b. Also, please see your individual comments for more detailed information of specific issues.

Cheers,
Adam

stephenf7072 · 2020-08-05T05:51:37Z

Nice one Adam. I've been over the new bits and apart from the minor omission of a "HZ" in keywords, no further comments from me.

oclyke · 2020-08-19T16:25:08Z

Hi @adamgarbo and @stephenf7072

Thanks for all your effort! You both probably know that we are in the midst of a big transition to the v2.0.0 core. That being said I think this PR should be merged into v2.0.0 rather than trying to squeeze it in on the tail end of v1.x.x

This PR is not forgotten!

adamgarbo · 2020-08-19T16:47:26Z

Thanks @oclyke

Is there a way to redirect which branch this PR will merge with? Please let me know if there's anything I can (or should) do.

Also, would you like me to create a new RTC library PR for v2.0.0 with the new time variable order (i.e. hund, sec, min, hour, etc.)?

Cheers,
Adam

oclyke · 2020-08-19T18:28:18Z

@adamgarbo I have already put the RTC library w/ the order changes into the v2.0.0 branch (took the liberty to do so cause I didn't want to bug anyone for help with it)

~~Yes, I will change the target of this PR - then perhaps I can get around to testing it and make it part of v2.0.0 rather than 2.x.x~~

Edit: actually I think you need to change the target branch. Should be fairly simple / intuitive.... lmk if you have trouble

adamgarbo · 2020-08-19T18:31:31Z

@oclyke Well, that was easy! Updated base branch to v2.0.0. I assumed that was the correct one, and not the preview?

oclyke · 2020-08-19T18:35:30Z

Yup! There is a sort of rolling release going on like this:

develop stuff in v2.0.0
merge into spoof-dev to trigger CI builds
pull changes and merge spoof-release-candidate into v2.0.0 to be back on track
make a snapshot of v2.0.0 in v2.0.0-preview for people to use w/o my changes breaking everything

By the way - there will be a decent amount of work to do to make this compatible with v2.0.0 - there is a completely different file structure and philosophy. I'll be here to lend a hand

adamgarbo · 2020-08-19T18:37:57Z

Thanks, Owen.

Is there any documentation on the new file structure and philosophy? Keen to learn more and happy to make changes as necessary.

Cheers,
Adam

oclyke · 2020-08-19T18:38:13Z

A quick glance at the new diffs makes me think..... it would probably be easiest to start from scratch based on the v2.0.0 branch. You could of course make a local copy of the meat + potatoes files of your PR.

oclyke · 2020-08-19T19:21:26Z

Wow - you're quick. Didn't see your last request for docs.

I don't have anything official to hand you. Maybe you will be able to tell just by looking at the file structure. Also possible that you won't be affected if you are just writing a library. Here are some notes:

run git submodule update --init --recursive1 in the core repo to make sure all the submodules exist for you
standard Arduino API functions that can be handled by mbed go in cores/arduino/mbed-bridge (which is its own repo)
standard Arduino API functions that rely on the Ambiq HAL go in cores/arduino/sdk
generally I am matching implementation files with their respective header files - even when it means having two c or cpp files with the same name in different core locations
license notices are shortened and just point at the LICENSE.md file in the core

There's probably a lot more - that's all for now though

oclyke · 2020-08-27T18:43:11Z

@adamgarbo - we've decided to do another support release on the v1.x core, so I will test this out and try to pull it in. That way we can have a reference for what we should be doing on the v2.0.0 version

adamgarbo · 2020-08-27T19:02:00Z

Hi @oclyke,

Sounds good! Please let me know if you need any assistance or clarification.

I haven't yet had a chance to rewrite the WDT library, but if the v2.0.0 RTC library is any example, it shouldn't be too much work. I noticed some of the changes you mentioned (e.g. #ifndef _APOLLO3_LIBRARIES_RTC_H_ and class Apollo3RTC {}).

I'll try to submit a PR to v.2.0.0 in the next couple of days. What's your optimistic timeframe for its release?

Cheers,
Adam

oclyke · 2020-08-27T19:07:46Z

@adamgarbo I just tried out these examples. First of all (I hope I have said this before) thanks for the great work! Your examples are well laid out and straight to the point.

The first example worked well. I then went on to test examples 2 and 3, but saw no output. Stumped, I went back to example 1 (basic) and saw no output any more. Moving on I even went to an unrelated example (pure serial) I still saw no output.

I haven't looked into it any deeper (using a debugger, for example) but can you confirm that you do not have similar issues?

adamgarbo · 2020-08-27T19:24:18Z

Hi Owen,

I initially tested all examples on a SparkFun Artemis Redboard. I just now tested them on an Edge 2 and they all appeared to be working correctly. Please see below for their respective outputs:

Example 1

15:18:15.045 -> Artemis Watchdog Timer Example
15:18:19.215 -> Interrupt: 1 Period: 4122 ms 
15:18:23.277 -> Interrupt: 2 Period: 4067 ms 
15:18:27.386 -> Interrupt: 3 Period: 4065 ms 
15:18:31.429 -> Interrupt: 4 Period: 4067 ms 
15:18:35.523 -> Interrupt: 5 Period: 4064 ms 
15:18:35.523 -> Warning: Watchdog has triggered a system reset

Example 2

15:18:55.097 -> �Artemis Watchdog Timer Example
15:18:56.357 -> Interrupt: 1 Period: 1007 ms
15:19:00.393 -> Interrupt: 2 Period: 4019 ms
15:19:04.480 -> Interrupt: 3 Period: 4066 ms
15:19:08.549 -> Interrupt: 4 Period: 4066 ms
15:19:12.639 -> Interrupt: 5 Period: 4068 ms
15:19:16.714 -> Interrupt: 6 Period: 4064 ms
15:19:20.790 -> Interrupt: 7 Period: 4064 ms
15:19:24.855 -> Interrupt: 8 Period: 4065 ms
15:19:28.954 -> Interrupt: 9 Period: 4065 ms
15:19:28.954 -> Warning: Watchdog has triggered a system reset

Example 3

15:20:16.623 -> �Artemis Watchdog Low Power Example
15:20:16.877 -> ⸮Watchdog interrupt: 2020-08-01 00:00:10.11
15:20:27.015 -> ⸮Watchdog interrupt: 2020-08-01 00:00:20.15
15:20:37.042 -> ⸮Watchdog interrupt: 2020-08-01 00:00:30.20
15:20:47.074 -> ⸮Watchdog interrupt: 2020-08-01 00:00:40.26
15:20:57.122 -> ⸮Watchdog interrupt: 2020-08-01 00:00:50.31
15:21:07.192 -> ⸮Alarm interrupt: 2020-08-01 00:01:00.01
15:21:16.872 -> ⸮Watchdog interrupt: 2020-08-01 00:01:10.04

I could have sworn we fixed that issue with the Artemis going to sleep and waking up without producing a question mark in the serial output. @nseidle did we only fix this on the OpenLog Artemis?

Cheers,
Adam

oclyke · 2020-08-27T19:56:37Z

Thanks - I will look more closely at my setup to see if something is going wrong.

I also thought we had fixed the UART startup issue. If I recall correctly it had to do with configuring the UART pins after the UART was initialized...

cores/arduino/am_sdk_ap3/mcu/apollo3/hal/am_hal_uart.h

oclyke · 2020-08-29T07:57:09Z

Alright, I think I found the issue and it is totally unrelated to this library. It is described in issue #254

I don't believe this is at all related to this PR, and it doesn't occur with the SVL loader either (hence why most people are unlikely to experience the issue)

nseidle and others added 22 commits June 26, 2020 13:51

Create keywords.txt

2ca5c57

Correct typo in HAL

9072743

Add checking of RTS/CTS pins to enable flow control. Fix typo in _beg…

ddda09e

…in().

Merge pull request #230 from sparkfun/addFlowControl

16894c2

Merge pull request #191 from sparkfun/addKeywords

33bd2d9

Create keywords.txt

Merge pull request #232 from sparkfun/release-candidate

1287ee0

Merge pull request #234 from sparkfun/release-candidate

2cc5ab4

Merge pull request #1 from sparkfun/master

0af5eb0

Bringing up to date with release1.1.2

Create Example8_ResetsAndWatchdog.ino

847eb3d

Create WDT.h

38d783d

Create WDT.cpp

d913933

Create library.properties

f8d1fb9

Create keywords.txt

fd8e357

Revert "Removed code using <time.h> non-core library (ie. Epoch stuff)"

57c81bd

This reverts commit ece522d.

Update WDT.h

66b6e6e

Update WDT.cpp

331b8d7

rename example

453196d

this keeps room around for the legacy BLE led example

rename the actual file

03ae640

Merge pull request #241 from stephenf7072/master

d9b3290

Add examples

e1a7d9b

Add WDT configuration

73a813b